Skip to content

fix: correct InvalidCustomPropertyError code and MemoryStore promise handling#2853

Merged
WilliamBergamin merged 1 commit into
slackapi:mainfrom
KirobotDev:main
Apr 8, 2026
Merged

fix: correct InvalidCustomPropertyError code and MemoryStore promise handling#2853
WilliamBergamin merged 1 commit into
slackapi:mainfrom
KirobotDev:main

Conversation

@KirobotDev
Copy link
Copy Markdown
Contributor

This PR fixes two bugs:

  1. InvalidCustomPropertyError: The error class was incorrectly using the wrong error code and now correctly uses the appropriate one.

  2. MemoryStore.get(): Added missing return statements after resolve() and reject() calls to prevent promise double settlement and ensure proper async flow control.

Changes

  • Fixed error code assignment in the InvalidCustomPropertyError class
  • Added return statements in MemoryStore.get() promise handlers to prevent multiple resolutions/rejections

Requirements

  • I've read and understood the Contributing Guidelines and have done my best effort to follow them.
  • I've read and agree to the Code of Conduct.

…handling

- InvalidCustomPropertyError now uses ErrorCode.InvalidCustomPropertyError instead of AppInitializationError
- MemoryStore.get() adds return statements after resolve/reject to prevent promise double settlement
@KirobotDev KirobotDev requested a review from a team as a code owner April 8, 2026 13:18
@WilliamBergamin WilliamBergamin added this to the 4.8.0 milestone Apr 8, 2026
@WilliamBergamin WilliamBergamin added bug M-T: confirmed bug report. Issues are confirmed when the reproduction steps are documented semver:patch labels Apr 8, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 8, 2026

Codecov Report

❌ Patch coverage is 66.66667% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 93.59%. Comparing base (41060d1) to head (171bad7).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/errors.ts 0.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2853   +/-   ##
=======================================
  Coverage   93.59%   93.59%           
=======================================
  Files          44       44           
  Lines        7853     7855    +2     
  Branches      686      687    +1     
=======================================
+ Hits         7350     7352    +2     
  Misses        498      498           
  Partials        5        5           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown
Contributor

@WilliamBergamin WilliamBergamin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are some nice finds 💯 both of these seem to be legit bugs 🥇

Thank you for your contribution to find and fix these 🙏

@WilliamBergamin WilliamBergamin merged commit a18c359 into slackapi:main Apr 8, 2026
26 checks passed
@KirobotDev
Copy link
Copy Markdown
Contributor Author

KirobotDev commented Apr 8, 2026

Hey thank you very much for accepting my pull request :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug M-T: confirmed bug report. Issues are confirmed when the reproduction steps are documented cla:signed semver:patch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants